Skip to content

fix(form-core, react-form): remove overwritting of falsy values upon array element shifting - #1440

Closed
juanvilladev wants to merge 9 commits into
TanStack:mainfrom
juanvilladev:fix/1439_falsy_values_overwritten_on_array_shifting
Closed

juanvilladev wants to merge 9 commits into
TanStack:mainfrom
juanvilladev:fix/1439_falsy_values_overwritten_on_array_shifting

Conversation

@juanvilladev

@juanvilladev juanvilladev commented Apr 21, 2025 •

Copy link
Copy Markdown
Contributor

This PR addresses two issues:

#1363: Temporary undefined field.state.value during React re-renders

#1439: Falsy values being overwritten when field/form stores go out of sync

  1. Fix for Falsy values are being turned into undefined upon removing array field #1439

When array operations like remove, shift, or swap are performed, the form and field stores can temporarily go out of sync. Previously, we were overwriting all falsy values (e.g., 0, false, '') during this sync process.

To fix this, I added a ternary check to preserve the original value when it's falsy but not undefined. This prevents unintended overwrites of valid falsy inputs.

  1. Fix for Non-index keys in React causing inputs to be uncontrolled when removing array fields #1363

React was re-rendering while the field store was briefly out of sync with the form store, leading to field.state.value being undefined. During this transitional state, we now return null from the field component, which avoids runtime errors and prevents rendering invalid state.

Additionally, we were encountering an error reading errorMap from undefined. This was due to how we retrieved field names within form-level validators.

The fix includes:

Switching to baseStore instead of the derived store when accessing the form state (from validators). This is because baseStore is the true source of truth and reflects updates immediately, even during field deletion. This ensures we always have an up-to-date list of active fields.

Let me know if anything needs more clarification or if you'd like to walk through any part of the solution!

… deleting array field

This commit fixes TanStack#1439 which will preserve existing falsy values within the update function when nameHasChanged is true
@nx-cloud

nx-cloud Bot commented Apr 21, 2025 •

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit b79e4b2.

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 3m 1s View ↗
nx run-many --target=build --exclude=examples/** ✅ Succeeded 23s View ↗

☁️ Nx Cloud last updated this comment at 2025-05-05 21:08:14 UTC

@pkg-pr-new

pkg-pr-new Bot commented Apr 21, 2025 •

Copy link
Copy Markdown
More templates

@tanstack/angular-form

npm i https://pkg.pr.new/@tanstack/angular-form@1440

@tanstack/form-core

npm i https://pkg.pr.new/@tanstack/form-core@1440

@tanstack/lit-form

npm i https://pkg.pr.new/@tanstack/lit-form@1440

@tanstack/react-form

npm i https://pkg.pr.new/@tanstack/react-form@1440

@tanstack/svelte-form

npm i https://pkg.pr.new/@tanstack/svelte-form@1440

@tanstack/solid-form

npm i https://pkg.pr.new/@tanstack/solid-form@1440

@tanstack/vue-form

npm i https://pkg.pr.new/@tanstack/vue-form@1440

commit: b79e4b2

@codecov

codecov Bot commented Apr 21, 2025 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.82%. Comparing base (8634cee) to head (b79e4b2).
⚠️ Report is 132 commits behind head on main.

Files with missing lines Patch % Lines
packages/form-core/src/FieldApi.ts 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1440      +/-   ##
==========================================
- Coverage   88.98%   88.82%   -0.16%     
==========================================
  Files          31       31              
  Lines        1398     1405       +7     
  Branches      353      354       +1     
==========================================
+ Hits         1244     1248       +4     
- Misses        137      140       +3     
  Partials       17       17              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

juanvilladev and others added 3 commits April 21, 2025 20:39
…olled when removing array fields TanStack#1363

This commit updates React to not render component when in transitory state of stores being out of sync due to array shifting
…Store as the source of logic instead of derived store.
@juanvilladev

Copy link
Copy Markdown
Contributor Author

Completed all updates, I believe this is ready for review.

  • Confirmed we no longer get the issue with uncontrolled components
  • Confirmed we no longer get the error regarding reading errorMap from undefined from validators
  • Confirmed falsy values are no longer being set to undefined upon deleting array field elements

@juanvilladev juanvilladev changed the title fix(form-core): remove overwritting of falsy values upon array element shifting fix(form-core, react-form): remove overwritting of falsy values upon array element shifting Apr 22, 2025

@Balastrong Balastrong left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The undefined check looks good, but let me challenge on the other fix: are you sure it's not a workaround to something that needs to be fixed on Store instead?

Also thank you for all the examples and details, great work as usual :)

@tmarnet

tmarnet commented May 28, 2025

Copy link
Copy Markdown
Contributor

Hello @Balastrong @juanvilladev 👋

I think a similar issue was reported recently that might be addressed by this PR?

I would be curious to know what the status on this fix is?

Thanks for everything 🙂

@juanvilladev

Copy link
Copy Markdown
Contributor Author

Won't be able to look into this for a while

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants